Repository navigation
FEAT: TVP support for async queries - #814
Subrata (subrata-ms) wants to merge 19 commits into
Conversation
Expose native TVP and SQL type-hint helpers lazily. Support iterable executemany batches and preserve result state on iteration failure. Add cursor closed-state reporting, document async result contracts, and cover these behaviors with async integration tests.
PR Performance Report✅ No regression detectedNo consistent slowdowns detected across all 2 environments. 0 IMPROVEMENTS 0 SLOWDOWNS 2/2 ENVIRONMENTS Coverage: 2 of 2 environments completed. Advisory result; does not block merging. Performance diagnosticsPhase times are inclusive diagnostics and must not be added together. They identify where measured time changed, not why it changed. No affected phases or call-count changes were recorded. All database tasks and timingsUnix / SQL Server 2022
Unix / SQL Server 2025
Build and measurement detailsPR head:
A consistent change requires more than 20% median paired movement, at least 1 ms between the median runtimes, and at least 80% of pairs exceeding the relative threshold in the same direction. A slowdown without enough pair agreement is reported as inconsistent. The displayed change is the median of paired before-and-after ratios. It is not recalculated from the two displayed median runtimes. Both revisions use profiling-enabled builds on the same agent and database, with alternating order and discarded warmups. Results are diagnostic and do not represent production-wheel latency. Raw samples and logs are attached to the ADO run as |
Subrata (subrata-ms)
changed the title
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Guard the optional native dependency test and complete the required PR metadata.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Adds preview async TVP/type-hint support, iterable executemany, cursor state reporting, and expanded integration tests.
Changes:
- Adds lazy native TVP and SQL type-hint exports.
- Supports synchronous iterable batches and failure-result preservation.
- Documents and tests async cursor/result behavior.
| File | Summary |
|---|---|
tests/AsyncTest/test_006_async_execute.py |
Tests iterable batches, TVPs, and type hints. |
tests/AsyncTest/test_005_async_cursor.py |
Tests cursor state and result behavior. |
tests/AsyncTest/test_004_async_logging.py |
Updates execution logging expectations. |
tests/AsyncTest/test_001_async_query_native.py |
Tests native exports. Finding: moderate, 2 votes—add an importorskip guard for the optional dependency at lines 55 and 65. |
mssql_python/async_query/async_execute.py |
Adds iterable batch execution and failure handling. |
mssql_python/async_query/async_cursor.py |
Adds cursor state and result-contract documentation. |
mssql_python/async_query/__init__.py |
Adds lazy native exports. Finding: nit, 1 vote—replace PR placeholders and empty summary with valid metadata. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Fix the indentation error that prevents the native async test module from being collected.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
Resolved since last review (1)
github-actions
Bot
added
pr-size: large
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/async_query/async_execute.pyLines 98-110 98 iteration_failed = False
99
100 def parameter_rows():
101 nonlocal iteration_failed
! 102 try:
! 103 yield from seq_of_parameters
! 104 except BaseException:
! 105 iteration_failed = True
! 106 raise
107
108 logger.debug(
109 "AsyncCursor.executemany: starting; use_prepare=%s",
110 use_prepare,📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.performance_counter.hpp: 0.7%
mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 62.6%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection_pool.cpp: 81.3%
mssql_python.pybind.connection.connection.cpp: 84.1%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.async_query.async_execute.py: 91.5%
mssql_python.pybind.fetch_temporal.hpp: 92.1%🔗 Quick Links
|
There was a problem hiding this comment.
🔵 Needs a closer look
Native-backed async behavior and live SQL integration require final human validation despite no confirmed defect.
0 open findings
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Gaurav Sharma (bewithgaurav)
left a comment
Gaurav Sharma (bewithgaurav)
left a comment
There was a problem hiding this comment.
For context, I have narrowed #789 to the shared TableValuedParameter definition, synchronous ODBC streaming, and sync documentation/tests. I removed the async adapter and async TVP tests from that PR to avoid duplicating the integration here.
The design direction remains one public TVP value with separate ODBC and TDS implementations. Could we build the async support here around that shared class, while retaining the native work already in this PR?
Shared interface
- Use
mssql_python.TableValuedParameter(type_name, columns, rows, *, schema=None)as the application-facing value. Keep_TableValuedParameterinternal. - Preserve the shared column format
(name, sql_type[, size_or_precision[, scale]]), positional binding, and explicit metadata. Translate this into native column triples insideasync_execute.py, including the shared0 = MAXconvention. - Preserve the existing identity and empty-input rules:
schema.Typeor a separateschemaargument;[]for an empty table;Nonefor nullable cells, not for the table itself. Reuse the shared validation rather than adding a parallel contract.
Initial async scope
- Start with materialized list/tuple rows. Reject synchronous iterators before consumption rather than silently materializing them. Native streaming remains a separate follow-up.
- Support the shared value through
execute(), including named parameters, surrounding scalars and multiple TVPs. Keep TVPexecutemany()and TVPsetinputsizes()overrides outside this initial slice; this does not affect ordinary non-TVP batch support. - Document the current Rust
0.3.0metadata restrictions: DECIMAL/NUMERIC(18,10) and temporal scale 7. Broader support needs a native fix and dependency update, not silent changes to the caller's declarations.
Landing and coverage
- Land the shared definition through #789, then integrate the async adapter here without copying the class. The independent cursor and type-hint improvements do not need to be rewritten.
- Use the shared wrapper in customer examples and async acceptance tests. Mirror the applicable cases in
tests/test_028_table_valued_parameters.py: empty/NULL input, supported types, Unicode/binary/MAX values, mixed parameters, reuse and error recovery. - Include rejection-before-consumption and caller-input preservation. Keep native-constructor tests as internal coverage and large memory workloads in stress coverage.
This keeps the implementation ownership separate while giving callers the same TVP interface.
| except BaseException: | ||
| iteration_failed = True | ||
| raise |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
|
||
|
|
||
| def test_async_tvp_accepts_numeric_and_temporal_column_metadata(): | ||
| from mssql_python.async_query import _TableValuedParameter |
| if not iteration_failed: | ||
| _reconcile_failed_execution(cursor, error) | ||
| raise |
Work Item / Issue Reference
Summary
This pull request introduces several improvements and enhancements to the
mssql_python.async_queryasync API, focusing on better native type integration, more robust and consistent cursor behavior, and improved handling of parameter iterables inexecutemany. It also expands the test suite to cover these changes and edge cases.Key highlights include:
executemany.closedproperty.Native type integration and exports
SQL_MONEY,SQL_SMALLMONEY,SQL_XML,SQL_JSON,SQL_VECTOR) and the internal_TableValuedParameterconstructor, with dynamic loading and error reporting if the native dependency is missing. These are now included in__all__anddir()as appropriate.AsyncCursor API and behavior improvements
closedproperty to async cursors to reflect both wrapper and parent connection state, with tests for idempotency and parent connection closure propagation.nextset,close,description, androwcountto document async-specific behaviors and differences from the synchronous API.nextsetcorrectly tracksrowcountanddescriptionper result, including edge cases.Executemany and parameter iterable handling
executemanysignatures to accept any synchronous iterable (not just sequences), improved error handling for non-iterables, and ensured iterables are consumed only once. Streaming/asynchronous iterables are explicitly not supported.Logging and test adjustments
batch_countforexecutemanyand adjusted test expectations accordingly.These changes make the async query API more robust, Pythonic, and consistent with both native and DB-API expectations.